fix(client): reject a mismatched sampleRate or format instead of ignoring it - #953
fix(client): reject a mismatched sampleRate or format instead of ignoring it#953chinmayv095 wants to merge 2 commits into
Conversation
|
I see that I wrote the original issue a bit vague - sorry for that 🤔 Not a huge fan of our SDK swallowing this. If a user pass in a specific |
PR SummaryMedium Risk Overview WebSocket Tests cover throw-on-mismatch, successful device switch when format matches, and Reviewed by Cursor Bugbot for commit 10733c9. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Correct, and fixed in 147f731. Ignoring was the wrong direction. sampleRate and format are things the developer is explicitly in control of, and if the value they asked for cannot be applied, swallowing that silently is the actual bug, not the inconsistency by itself. Both connection types now throw on a value they cannot honor instead. The throw is on the value, not the field name.
4 tests updated in Fail-first with only the three source files stashed: the 3 tests asserting the new throw fail (2 pass regardless, since they test the unchanged no-op case). |
chinmayv095
changed the title
changeInputDevice forwards sampleRate, format and preferHeadphonesForIosDevices to the input controller. The WebSocket path ignores all three, because they cannot be applied to an already-running AudioContext, and switches the device. The WebRTC path threw instead, so the same call switched the microphone on one connection type and failed outright on the other, and the device was never changed. The output controllers differed the same way for sampleRate and format. Both WebRTC controllers now ignore the options they cannot apply, which is what makes the two connection types substitutable behind InputController and OutputController.
…ring it @kraenhansen pointed out that ignoring an explicit sampleRate/format on setDevice hides a request the SDK cannot honor, when the caller has given a clear signal we cannot match. Both connection types now throw when the requested value differs from the one the connection already negotiated, on WebRTC as well as WebSocket, input and output. Re-passing the connection's own current sampleRate/format alongside a device id, which callers do routinely and which an existing test in index.test.ts already exercised, stays a no-op: the value is not actually being changed, so there is no unmatched intent to reject. preferHeadphonesForIosDevices stays a silent best-effort hint on WebRTC, unchanged, since it was not part of this complaint and is not a format guarantee the way sampleRate/format are.
147f731 to
10733c9
Compare
2 participants
Fixes #549.
The premise still holds, under a different name
The issue names
setInputDevice, which no longer exists — the public entry point is nowVoiceConversation.changeInputDevice()(anduseConversation().changeInputDevice()in@elevenlabs/react), delegating toInputController.setDevice. Verified againstmain: the inconsistency it describes is unchanged.changeInputDeviceacceptsPartial<FormatConfig> & InputDeviceConfigand forwardssampleRate,formatandpreferHeadphonesForIosDevicesstraight through:MediaDeviceInput.setDevice) ignoredsampleRate/formatunconditionally — they cannot be applied to an already-runningAudioContext— and switched the device.WebRTCConnection.input.setDevice) threw onsampleRate/format/preferHeadphonesForIosDevicesunconditionally, and did not change the device at all.The output controllers had the same asymmetry for
sampleRate/format.What this does
Both connection types now compare the requested
sampleRate/formatagainst the one the connection actually negotiated (the WebSocket path's own configured values; WebRTC's fixedpcm_48000), input and output:sampleRate/formatare things the caller explicitly controls; silently ignoring a value we cannot honor was the actual bug in the original WebSocket behavior, not just the WebRTC/WebSocket inconsistency by itself. See this thread for the full reasoning after @kraenhansen's review.index.test.tsalready had a test assertingchangeInputDevice({ sampleRate: 16000, format: "pcm" })and the output equivalent succeed with no deviceId, passing the session's own format back rather than asking to change it. That existing pattern still works.preferHeadphonesForIosDevicesstays a silent best-effort hint on WebRTC (unchanged from before): it's a device-selection preference, not a format guarantee the waysampleRate/formatare.Tests
WebRTCConnection.test.ts: 4 tests asserting throw on a real mismatch (input sampleRate, input format, output sampleRate+format, each with a device id present), 2 tests asserting the re-passed-same-value case still switches the device.input.test.ts/output.test.ts(WebSocket path): throw-on-mismatch and no-op-on-match, exercised directly againstMediaDeviceInput/MediaDeviceOutput.output.test.tsis new —MediaDeviceOutputhad no test coverage before, and itscreate()needs a liveAudioContext/AudioWorkletthat isn't available outside this repo's browser test project (onlyindex.test.tsandinput.test.tsare configured to run there). The guard runs beforesetDevicetouches any instance state, so it's tested against the class's realsetDevicewithout going throughcreate(), avoiding adding output creation to the browser suite for this fix alone.Fail-first with only the three source files stashed: the 3 throw-assertion tests fail (rest are unaffected, since they test the no-op path).
packages/client227/227 with the fix restored.packages/react140/140, unaffected.turbo lint/check-types/buildgreen across all 9 packages (29/29). Changeset included,patchon@elevenlabs/client.No test pinned the old throwing/ignoring behaviour as-is, and nothing in
packages/reactorpackages/react-nativeneeded changing — both call straight through.Heads-up on overlap: #642, #813 and #814 also touch
WebRTCConnection.ts, but none of them touches eithersetDevice, and the hunks here are small and adjacent.